HDDS-14259. Improve TestManagedDirectSlice. #9571
Conversation
|
@szetszwo thanks for the patch! |
sreejasahithi
left a comment
sreejasahithi
left a comment
There was a problem hiding this comment.
Thanks @szetszwo for working on this. LGTM
swamirishi
left a comment
swamirishi
left a comment
There was a problem hiding this comment.
@szetszwo thanks for working on the patch I have left a comment inline about the test coverage.
| final byte[] bytes = new byte[size]; | ||
| RANDOM.nextBytes(bytes); | ||
| try (CodecBuffer buffer = CodecBuffer.allocateDirect(size).put(ByteBuffer.wrap(bytes)); | ||
| ManagedDirectSlice directSlice = new ManagedDirectSlice(buffer.asReadOnlyByteBuffer()); |
There was a problem hiding this comment.
We should also test a byte buffer with a non zero position to test the slice initiailization in the constructor. Consider the case if someone removes the slice logic then things would break without a test case failure. If there is some logic change we should ensure we have test case for that.
BTW I have raised a patch on rocksdb which gives us a way to avoid the slicing and with this constructor we can directly pass the position and remaining to the JNI layer in a single call.
facebook/rocksdb#14208
There was a problem hiding this comment.
Thanks for making the changes to the patch @szetszwo LGTM
|
@rich7420 , @sreejasahithi , @swamirishi , thanks a lot for reviewing this! |
4 participants
What changes were proposed in this pull request?
The offset, which is not useful anymore after HDDS-14237, should be removed.(Offset is still useful)What is the link to the Apache JIRA
HDDS-14259
How was this patch tested?
This is a test only change.